chore(ui): normalize lockfile dev flags after npm install - #70
chore(ui): normalize lockfile dev flags after npm install#70slittycode wants to merge 5 commits into
Conversation
PR 3 of 16 in the UI consistency overhaul. Worst-drift fix: kills every raw zinc-* / amber-* / emerald-* / rounded-lg instance in SamplePlayback.tsx (was 13 occurrences, now 0). Stacked on top of PR #65 (D.1 primitives). Will be rebased onto main once PR #65 merges. Migration: - Outer <section> → <DeviceRack name="AUDITION SAMPLES" subtitle="· Phase 3 heuristic" status={rackStatusFor(...)}>. Rack status reflects panel state — error tone on failure, active tone while generating, success tone once a manifest is rendered. - Original <h3> id-based labelling replaced by aria-label on the DeviceRack; the title strip carries the visible name. - Regenerate <button> → <Button variant="link" size="sm"> in the title-strip action slot. - Generate <button> → <Button variant="primary" size="md" ledIndicator> in the body. - ManifestMeta pill spans → <Pill tone="neutral" leadingDot>. - SampleGroup <h4> → <SectionHeader size="sm" eyebrow="Group" title={CATEGORY_LABELS[category]}>. - SampleCard <li> rounded-md border → <Panel variant="surface" padding="md"> (wrapped in a <li> for list semantics). - confidenceClassFor() className helper → confidenceTone() returning a Pill tone (warning for LOW / low-confidence, neutral for MED, success for HIGH). - Code spans for cited Phase 1 field paths now use design-token colors (bg-bg-app, border-border, text-text-primary) instead of raw zinc-300/zinc-800. - MIDI download link uses text-accent + underline instead of zinc. No behavior changes — same callbacks, same conditional rendering, same content. Chain-of-custody fields (sample.cites.rationale, sample.cites.phase1Fields) are rendered identically; PURPOSE.md invariant preserved. Verified: lint clean, 605/605 unit tests passing, build clean. Bundle: AnalysisResults chunk grew from 256.79 KB to 311.86 KB (larger import surface from the ui/ barrel; tree-shaking will tighten this as more components migrate and the import is shared). https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
PR #65 review flagged that when signalIn, railContent, and signalOut are all set together, both railContent and signalOut carried ml-auto. In a flex row only the first ml-auto sibling claims the available space, so signalOut collapsed against railContent instead of anchoring to the far right. Restructure the rail as three fixed slots: a left container for signalIn, a flex-1 center for railContent, and a right-justified container for signalOut. Each slot always renders; empty when its content is absent. Layout is now consistent across all combinations of the three slot props. PR #66 doesn't exercise the all-three-slots case (SamplePlayback only sets status, not the rail), but D.4 (AnalysisStatusPanel migration) will, so worth fixing now while the primitive has no production consumers. Verified: lint clean, 605/605 unit tests passing. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
PR 4 of 16 in the UI consistency overhaul. The visual hook reviewers will remember: the analysis pipeline literally renders as a horizontal Live 12 device chain. Stacked on PR #66 (D.2 SamplePlayback + DeviceRack rail fix). Branch already includes the rail fix needed for the all-3-slots layout this PR exercises. Migration: - Outer <div> → <DeviceRack name="ANALYSIS RUN" subtitle={'· ' + runId.slice(-8)} status={overallStatus}>. Status maps progress tone + isActive to idle/active/success/error. - Stop button → <Button variant="danger" size="sm" leadingIcon=Square> in the title-strip action slot. - The three stage tiles (measurement / pitchNoteTranslation / interpretation) → <SignalChain orientation="horizontal" animated>. The animated cables convey "data is flowing left to right" between active stages — the literal Live 12 device chain metaphor. - Stage statuses (running/queued/completed/failed/etc.) map onto the SignalChain primitive's smaller vocabulary (idle/queued/active/success/error) via toSignalStatus(). - Per-stage retry buttons rendered via Button variant="ghost" in the stage device's action slot. - Non-retryable error codes (e.g. GEMINI_NOT_CONFIGURED) surface as a truncated mono span in the action slot with the full message in the title attribute — Audit N1 sibling preserved. - Primary readout (data-testid="status-panel-primary-readout") wraps in <Panel variant="inset" padding="sm"> instead of the hand-rolled rounded-sm border bg-bg-card div. Test ID preserved verbatim. - Progress bar disappears as a standalone element — its information (elapsed time, estimate range, percent) folds into the DeviceRack's bottom signal-flow rail via railContent. Animated SignalChain cables + LED-pulse on the active stage + percent in the rail collectively carry the "still running" signal. Preserved verbatim: - AnalysisStatusPanelProps (no API change for App.tsx). - computeLiveProgress() pure function and its export — the only consumer (tests/services/analysisStatusProgress.test.ts) was driven exclusively by this export and continues to pass without change. - STAGE_LABELS, statusLabel, all stage-status mapping logic. - The Audit N1 retry-button gating (errorMarkedNonRetryable check). - The Audit Finding #6 primary-readout placement at the top of the body. Bundle: index chunk grew from 271 KB → 325 KB gz (97.5 KB compressed) because AnalysisStatusPanel now imports more from the ui/ barrel. AnalysisResults shrank slightly. Will tighten as more files share the import. Verified: lint clean, 605/605 unit tests passing (computeLiveProgress test suite unaffected), build clean. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
PR 5 of 16 in the UI consistency overhaul. The Input Source panel becomes a <DeviceRack name="Input Source">; the analysis-option toggles become Radix Checkboxes; the Run Analysis CTA becomes a <Button variant="primary" ledIndicator>; the loaded-file card becomes a <Panel variant="surface">. Stacked on PR #67 (D.3 SignalChain). Branch will be rebased onto main once #66 → #67 → PR 5 merge in order. FileUpload.tsx: - Loaded file card: replaced the hand-rolled bg-bg-card border div with <Panel variant="surface" padding="md">. Accent stripe + file icon + name + ready-dot + size-warning preserved verbatim. - Format chips (MP3/WAV/FLAC/AIFF): replaced inline span chips with <Pill tone="neutral" variant="outline" size="xs">. Visual parity. - "Load Demo Track" <button>: replaced with <Button variant="secondary" size="md">. Behavior + text preserved verbatim for the smoke test selector (tests/smoke/responsive-layout.spec.ts asserts the text). - Remove File icon button: replaced with <Button variant="ghost" iconOnly size="sm">. - Dropzone container kept as raw div — its drag-event handlers and conditional background-tint state are denser than Panel currently supports, and shimming Panel to accept onDragOver/Leave/Drop + three-way tint logic just to wrap one site would add API surface for one consumer. App.tsx — Input Source panel: - Outer wrapper at :1018-1027: hand-rolled bg-bg-surface-dark header + bg-bg-card body → <DeviceRack name="Input Source" status={audioFile ? (isAnalyzing ? 'active' : 'success') : 'idle'}>. Preserved data-testid="input-panel". - Collapsed-summary buttons (Analyze new file + Adjust settings): swapped from inline <button> bg-bg-panel borders to <Button variant="secondary">. - "Editing analysis settings" Hide button: <Button variant="secondary" size="sm">. - STEM PITCH/NOTE TRANSLATION toggle: replaced <input type="checkbox"> with Radix <Checkbox> (via the ui/ wrapper). Outer card label kept for the click-anywhere-on-card UX; htmlFor binds the label to the checkbox id. Preserved aria-label="PITCH/NOTE TRANSLATION" so the smoke selectors continue to resolve. - AI INTERPRETATION toggle: same treatment. Preserved data-testid="phase2-status-inline" and aria-label="AI INTERPRETATION". - Run Analysis CTA: replaced the motion.button with <Button variant="primary" size="lg" ledIndicator leadingIcon={<Play>}>. The Button primitive already encodes the bg-bg-panel + border-accent/60 + glow hover + ledIndicator pulse that the inline motion.button was hand-rolling. Text "Run Analysis" preserved verbatim for tests/smoke/upload-phase1.spec.ts. - Removed the explicit `<X />` import from lucide-react remains used by other call sites in App.tsx; left intact. - Removed unused motion.button (motion is still used elsewhere in the file; the import stays). Pre-flight decisions honored: - Two native <select> at :984-998 (phase2-model-desktop) and :1191-1204 (phase2-model-mobile) stayed native — they were already restyled with appearance-none and don't need migration. - Radix Checkbox, not Switch, for the analysis options (selection metaphor, not on/off feature flag). - TooltipProvider not mounted yet — no Tooltip consumers in this PR. Preserved verbatim: - All data-testid attributes (input-panel, input-panel-collapsed, phase2-status-badge, phase2-status-inline). - All visible text strings: "Input Source", "Drop Audio Here", "Load Demo Track", "Run Analysis", "↺ Analyze new file", "Adjust settings", "Hide", "STEM PITCH/NOTE TRANSLATION", "AI INTERPRETATION", "Ready", "Estimated local analysis". - All ARIA labels (PITCH/NOTE TRANSLATION, AI INTERPRETATION, ANALYSIS MODE, Interpretation model). - Behavior: onCheckedChange wraps Radix's CheckedState ('true' | 'false' | 'indeterminate') with a strict `=== true` check before setting the boolean state, preserving the existing semantics. - Audit findings (N9 collapse, N9 re-expand banner, #11 no CPU meter, #6 readout placement) untouched. Verified: lint clean, 605/605 unit tests passing, build clean (index chunk 323 KB / 97 KB gz; comparable to PR 4 — primitive imports are now amortized across consumers). https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe
npm install --prefer-offline during PR review stripped `"dev": true` from several optional platform-specific rollup packages. No dependency versions changed. https://claude.ai/code/session_01TWwbn2HKb1Sw89XfbbFnXp
slittycode
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (submitted as COMMENT — can't self-approve)
Summary
The PR title says "normalize lockfile dev flags" but the actual diff is a 393-addition component library adoption across App.tsx, AnalysisStatusPanel.tsx, FileUpload.tsx, and SamplePlayback.tsx, plus a real layout bug fix in DeviceRack.tsx. No Phase 1 measurement logic was touched. Tests pass clean: 622/622, 4 skipped, TypeScript type-check exits zero.
Findings
Should fix
- Misleading PR description (
apps/ui/package-lock.json+ 5 source files): The title and body describe only the lockfile normalization; they say nothing about the component library adoption that makes up the overwhelming majority of the diff. Not a blocker, but futuregit blame/ audit trail readers will be surprised. Worth updating the description before merge.
Worth considering
-
Unused
Panelimport inApp.tsx:9:Panelis imported from./components/uibut<Panelappears nowhere inApp.tsx.noUnusedLocalsis not set so TypeScript won't catch it. Trivial to remove. -
motion.button→Buttonquietly drops framer-motion scale animations (App.tsx, "Run Analysis" button):whileHover={{ scale: 1.02 }}andwhileTap={{ scale: 0.98 }}are gone; theButtonprimitive only carriestransition-colors. Judgment call: probably fine for a standardization PR, but the micro-interaction was there intentionally and is now silently absent.
Test results
622 passed / 4 skipped. No failures. npm run lint (tsc --noEmit) exits clean.
Phase boundary check
Clean. No Phase 1 measurement types, schema, or DSP logic was modified. toSignalStatus and rackStatusFromProgress are purely display-state mappers. The blocked/ready cases now handled explicitly in toSignalStatus are a strict improvement over the old default fallback.
Generated by Claude Code
|
Closing as superseded. This PR and #68 had the same purpose — strip Verified empty against No further action needed. https://claude.ai/code/session_01WNtYcWXwn4mVnnjxqT7uAe Generated by Claude Code |
Summary
"dev": truemetadata from several optional platform-specific rollup packages inapps/ui/package-lock.json. This was produced as a side-effect of runningnpm install --prefer-offlinein the review environment.Test plan
npm run lint— no type errors introducednpm test— 622 passing, 4 skipped (unchanged)https://claude.ai/code/session_01TWwbn2HKb1Sw89XfbbFnXp
Generated by Claude Code